Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Use rich Panel for showing warning in train_test_split #1086

Merged
merged 2 commits into from
Jan 10, 2025
Merged

feat: Use rich Panel for showing warning in train_test_split #1086

merged 2 commits into from
Jan 10, 2025

Conversation

glemaitre
Copy link
Member

closes #1060

It is the alternative to #1060 using rich. I added a test to check that we can filter the warning since we are not using the usual warnings module.

In the future, we could factor out the code in a utils to be sure that we can also transform the warnings into error.

@glemaitre glemaitre mentioned this pull request Jan 10, 2025
Closed
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Jan 10, 2025 •
edited
Loading

Documentation preview @ 1133dd0

@sylvaincom sylvaincom self-requested a review January 10, 2025 16:20
sylvaincom
sylvaincom approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@sylvaincom sylvaincom left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The display seems to work fine, many thanks!

sylvaincom
sylvaincom previously requested changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@sylvaincom sylvaincom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address failing tests

@github-actions GitHub Actions
Copy link
Contributor

Coverage

Coverage Report for backend
FileStmtsMissCoverMissing
venv/lib/python3.12/site-packages/skore
   __init__.py120100% 
   __main__.py8180%19
   exceptions.py30100% 
venv/lib/python3.12/site-packages/skore/cli
   __init__.py50100% 
   cli.py33385%104, 111, 117
   color_format.py43390%35–>40, 41–43
   launch_dashboard.py261539%36–57
   quickstart_command.py14750%37–51
venv/lib/python3.12/site-packages/skore/item
   __init__.py210100% 
   cross_validation_item.py1371093%27–42, 370
   item.py411368%85, 88, 92–112
   item_repository.py42293%12–13
   media_item.py70494%15–18
   numpy_array_item.py25193%15
   pandas_dataframe_item.py34195%15
   pandas_series_item.py34195%15
   polars_dataframe_item.py32194%15
   polars_series_item.py27194%15
   primitive_item.py27292%13–15
   sklearn_base_estimator_item.py33195%15
   skrub_table_report_item.py10186%11
venv/lib/python3.12/site-packages/skore/persistence
   __init__.py00100% 
   abstract_storage.py22195%130
   disk_cache_storage.py33195%44
   in_memory_storage.py200100% 
venv/lib/python3.12/site-packages/skore/project
   __init__.py30100% 
   create.py52888%116–122, 132–133, 140–141
   load.py23389%43–45
   open.py140100% 
   project.py64491%135, 149, 183, 187
venv/lib/python3.12/site-packages/skore/sklearn
   __init__.py40100% 
   find_ml_task.py35195%41–>49, 50
   types.py20100% 
venv/lib/python3.12/site-packages/skore/sklearn/_estimator
   __init__.py100100% 
   base.py76298%87–88
   metrics_accessor.py198298%131, 266
   report.py165197%145–>151, 147–>149, 150, 153–>155, 159–>163, 408–>413
   utils.py11110%1–19
venv/lib/python3.12/site-packages/skore/sklearn/_plot
   __init__.py40100% 
   precision_recall_curve.py126297%200–>203, 313–314
   prediction_error.py75099%289–>297
   roc_curve.py95394%156, 167–>170, 223–224
   utils.py770100% 
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation
   __init__.py20100% 
   cross_validation_helpers.py47490%104–>136, 123–126
   cross_validation_reporter.py35195%177
venv/lib/python3.12/site-packages/skore/sklearn/cross_validation/plots
   __init__.py00100% 
   compare_scores_plot.py29192%10, 45–>48
   timing_plot.py29194%10
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split
   __init__.py00100% 
   train_test_split.py36294%16–17
venv/lib/python3.12/site-packages/skore/sklearn/train_test_split/warning
   __init__.py80100% 
   high_class_imbalance_too_few_examples_warning.py17378%16–18, 80
   high_class_imbalance_warning.py18288%16–18
   random_state_unset_warning.py11187%15
   shuffle_true_warning.py9091%44–>exit
   stratify_is_set_warning.py11187%15
   time_based_column_warning.py22189%17, 69–>exit
   train_test_split_warning.py5180%21
venv/lib/python3.12/site-packages/skore/ui
   __init__.py00100% 
   app.py25571%24, 53–58
   dependencies.py7186%12
   project_routes.py500100% 
venv/lib/python3.12/site-packages/skore/utils
   __init__.py00100% 
   _accessor.py70100% 
   _logger.py21484%14–18
   _show_versions.py310100% 
venv/lib/python3.12/site-packages/skore/view
   __init__.py00100% 
   view.py50100% 
   view_repository.py16283%8–9
TOTAL222713693% 

Tests Skipped Failures Errors Time
361 0 💤 0 ❌ 0 🔥 44.565s ⏱️

@sylvaincom sylvaincom dismissed their stale review January 10, 2025 17:02

Looks good now

sylvaincom
sylvaincom approved these changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@sylvaincom sylvaincom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sylvaincom sylvaincom merged commit 6f6fafb into probabl-ai:main Jan 10, 2025
18 checks passed
thomass-dev pushed a commit that referenced this pull request Jan 13, 2025
@glemaitre @thomass-dev
feat: Use rich Panel for showing warning in train_test_split (#1086)
1323acb 
closes #1060 

It is the alternative to #1060 using `rich`. I added a test to check
that we can filter the warning since we are not using the usual
`warnings` module.

In the future, we could factor out the code in a utils to be sure that
we can also transform the warnings into error.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sylvaincom sylvaincom sylvaincom approved these changes

Assignees

@glemaitre glemaitre

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@glemaitre @sylvaincom